Skip to content

Conversation

@Prabhuk
Copy link
Contributor

@Prabhuk Prabhuk commented May 20, 2025

When the variable guared by a lock was enclosed in parenthesis, access
violation warnings were not emitted. This patch fixes it and adds a
regression test.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer clang:analysis labels May 20, 2025
@llvmbot
Copy link
Member

llvmbot commented May 20, 2025

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Prabhu Rajasekaran (Prabhuk)

Changes

When the variable guared by a lock was enclosed in parenthesis, access
violation warnings were not emitted. This patch fixes it and adds a
regression test.


Full diff: https://github.com/llvm/llvm-project/pull/140656.diff

2 Files Affected:

  • (modified) clang/lib/Analysis/ThreadSafety.cpp (+1-1)
  • (added) clang/test/Analysis/thread-safety-handle-parenthesis.cpp (+20)
diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index 7e86af6b4a317..156df51a71012 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1671,7 +1671,7 @@ void ThreadSafetyAnalyzer::checkAccess(const FactSet &FSet, const Expr *Exp,
         // Guard against self-initialization. e.g., int &i = i;
         if (E == Exp)
           break;
-        Exp = E;
+        Exp = E->IgnoreImplicit()->IgnoreParenCasts();
         continue;
       }
     }
diff --git a/clang/test/Analysis/thread-safety-handle-parenthesis.cpp b/clang/test/Analysis/thread-safety-handle-parenthesis.cpp
new file mode 100644
index 0000000000000..6be8362a650e0
--- /dev/null
+++ b/clang/test/Analysis/thread-safety-handle-parenthesis.cpp
@@ -0,0 +1,20 @@
+
+// RUN: %clang_cc1 -verify -fsyntax-only -std=c++20 -Wthread-safety %s
+
+class __attribute__((lockable)) Lock {};
+
+void sink_protected(int) {}
+
+class Baz {
+public:
+    Lock lock_;
+    int protected_num_ __attribute__((guarded_by(lock_))) = 1;
+};
+
+void baz_paran_test() {
+    Baz baz;     
+    int& n = baz.protected_num_;
+    sink_protected(n); // expected-warning{{reading variable 'protected_num_' requires holding mutex 'baz.lock_'}}
+    int& n2 = (baz.protected_num_);
+    sink_protected(n2); // expected-warning{{reading variable 'protected_num_' requires holding mutex 'baz.lock_'}}
+}

@llvmbot
Copy link
Member

llvmbot commented May 20, 2025

@llvm/pr-subscribers-clang-analysis

Author: Prabhu Rajasekaran (Prabhuk)

Changes

When the variable guared by a lock was enclosed in parenthesis, access
violation warnings were not emitted. This patch fixes it and adds a
regression test.


Full diff: https://github.com/llvm/llvm-project/pull/140656.diff

2 Files Affected:

  • (modified) clang/lib/Analysis/ThreadSafety.cpp (+1-1)
  • (added) clang/test/Analysis/thread-safety-handle-parenthesis.cpp (+20)
diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index 7e86af6b4a317..156df51a71012 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1671,7 +1671,7 @@ void ThreadSafetyAnalyzer::checkAccess(const FactSet &FSet, const Expr *Exp,
         // Guard against self-initialization. e.g., int &i = i;
         if (E == Exp)
           break;
-        Exp = E;
+        Exp = E->IgnoreImplicit()->IgnoreParenCasts();
         continue;
       }
     }
diff --git a/clang/test/Analysis/thread-safety-handle-parenthesis.cpp b/clang/test/Analysis/thread-safety-handle-parenthesis.cpp
new file mode 100644
index 0000000000000..6be8362a650e0
--- /dev/null
+++ b/clang/test/Analysis/thread-safety-handle-parenthesis.cpp
@@ -0,0 +1,20 @@
+
+// RUN: %clang_cc1 -verify -fsyntax-only -std=c++20 -Wthread-safety %s
+
+class __attribute__((lockable)) Lock {};
+
+void sink_protected(int) {}
+
+class Baz {
+public:
+    Lock lock_;
+    int protected_num_ __attribute__((guarded_by(lock_))) = 1;
+};
+
+void baz_paran_test() {
+    Baz baz;     
+    int& n = baz.protected_num_;
+    sink_protected(n); // expected-warning{{reading variable 'protected_num_' requires holding mutex 'baz.lock_'}}
+    int& n2 = (baz.protected_num_);
+    sink_protected(n2); // expected-warning{{reading variable 'protected_num_' requires holding mutex 'baz.lock_'}}
+}

@llvmbot
Copy link
Member

llvmbot commented May 20, 2025

@llvm/pr-subscribers-clang

Author: Prabhu Rajasekaran (Prabhuk)

Changes

When the variable guared by a lock was enclosed in parenthesis, access
violation warnings were not emitted. This patch fixes it and adds a
regression test.


Full diff: https://github.com/llvm/llvm-project/pull/140656.diff

2 Files Affected:

  • (modified) clang/lib/Analysis/ThreadSafety.cpp (+1-1)
  • (added) clang/test/Analysis/thread-safety-handle-parenthesis.cpp (+20)
diff --git a/clang/lib/Analysis/ThreadSafety.cpp b/clang/lib/Analysis/ThreadSafety.cpp
index 7e86af6b4a317..156df51a71012 100644
--- a/clang/lib/Analysis/ThreadSafety.cpp
+++ b/clang/lib/Analysis/ThreadSafety.cpp
@@ -1671,7 +1671,7 @@ void ThreadSafetyAnalyzer::checkAccess(const FactSet &FSet, const Expr *Exp,
         // Guard against self-initialization. e.g., int &i = i;
         if (E == Exp)
           break;
-        Exp = E;
+        Exp = E->IgnoreImplicit()->IgnoreParenCasts();
         continue;
       }
     }
diff --git a/clang/test/Analysis/thread-safety-handle-parenthesis.cpp b/clang/test/Analysis/thread-safety-handle-parenthesis.cpp
new file mode 100644
index 0000000000000..6be8362a650e0
--- /dev/null
+++ b/clang/test/Analysis/thread-safety-handle-parenthesis.cpp
@@ -0,0 +1,20 @@
+
+// RUN: %clang_cc1 -verify -fsyntax-only -std=c++20 -Wthread-safety %s
+
+class __attribute__((lockable)) Lock {};
+
+void sink_protected(int) {}
+
+class Baz {
+public:
+    Lock lock_;
+    int protected_num_ __attribute__((guarded_by(lock_))) = 1;
+};
+
+void baz_paran_test() {
+    Baz baz;     
+    int& n = baz.protected_num_;
+    sink_protected(n); // expected-warning{{reading variable 'protected_num_' requires holding mutex 'baz.lock_'}}
+    int& n2 = (baz.protected_num_);
+    sink_protected(n2); // expected-warning{{reading variable 'protected_num_' requires holding mutex 'baz.lock_'}}
+}

@Prabhuk Prabhuk requested a review from AaronBallman May 20, 2025 02:01
When the variable guared by a lock was enclosed in parenthesis, access
violation warnings were not emitted. This patch fixes it and adds a
regression test.
@Prabhuk Prabhuk force-pushed the fix_threadsafety_paranthesis branch from 74dbb15 to 8570d7a Compare May 20, 2025 02:03
@steakhal steakhal changed the title Thread Safety Analysis: Handle parenthesis [clang][analysis] Thread Safety Analysis: Handle parenthesis May 20, 2025
Copy link
Contributor

@steakhal steakhal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks.

@Prabhuk Prabhuk merged commit 1a9377b into llvm:main May 20, 2025
11 checks passed
@Prabhuk Prabhuk deleted the fix_threadsafety_paranthesis branch May 20, 2025 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:analysis clang:static analyzer clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants